Updated SnippetList new feature. #592
Conversation
|
This looks amazing ! Can you resolve the conflicts and I can merge? |
We still have a long way to go, |
Antoliny0919
force-pushed
the
new_feature_snippet_list
branch
from
6b64e5a to
ce65b33
Compare
Antoliny0919
force-pushed
the
new_feature_snippet_list
branch
from
ce65b33 to
c91e128
Compare
| {#<div id="sidebar" class="hidden md:block md:w-1/4">#} | ||
| {#<p>You're looking at the top-rated snippets currently on the site; {% if user.is_authenticated %}if you'd like to contribute, just click the ratings link in the sidebar of a particular snippet's page; you can rate any snippet "useful" or "not useful"{% else %}if you'd like to contribute, <a href="{% url 'account_login' %}">sign up for an account</a> and you'll be able to rate any snippet you see{% endif %}.</p>#} | ||
| {##} | ||
| {#<nav class="filter">#} | ||
| {# <h3>Filter by date</h3>#} | ||
| {# <ul>#} | ||
| {# <li{% if not months %} class="active"{% endif %}><a href="{{ request.path }}">Any time</a></li>#} | ||
| {# <li{% if months == 3 %} class="active"{% endif %}><a href="{{ request.path }}?months=3">3 months</a></li>#} | ||
| {# <li{% if months == 6 %} class="active"{% endif %}><a href="{{ request.path }}?months=6">6 months</a></li>#} | ||
| {# <li{% if months == 12 %} class="active"{% endif %}><a href="{{ request.path }}?months=12">1 year</a></li>#} | ||
| {# </ul>#} | ||
| {#</nav>#} | ||
| {##} | ||
| {#<p><a rel="alternate" href="{% url 'cab_feed_latest' %}" type="application/atom+xml"><i class="fa fa-fw fa-rss-square"></i>Feed of latest snippets</a></p>#} | ||
| {##} | ||
| {#</div>#} |
There was a problem hiding this comment.
The date filter has been commented out.
It will likely be removed.
Later, more detailed filtering options will be provided through a slide-in panel,
and for the date filter, I think it would be better to add two input fields and offer it in a between format.
I will also create a related issue for this.
There was a problem hiding this comment.
Issue -> #595
Antoliny0919
force-pushed
the
new_feature_snippet_list
branch
from
c91e128 to
83d7e61
Compare
Antoliny0919
force-pushed
the
new_feature_snippet_list
branch
from
83d7e61 to
f430f46
Compare
Antoliny0919
requested review from
Copilot and
chriswedgwood
and removed request for
Copilot
There was a problem hiding this comment.
Pull Request Overview
This PR redesigns the SnippetList interface from a traditional table layout to a modern card-based grid design using Django components. The changes modernize the UI with new fonts, color schemes, and responsive layout while replacing hardcoded HTML tables with reusable components.
- Replaces table-based snippet listings with a card-based grid layout using Django components
- Introduces new color variables and Google Fonts (Playfair Display, Nunito, Raleway) for improved typography
- Implements responsive design with conditional sidebar display and flexible content width
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| theme/static_src/src/styles.css | Adds new color variables and font definitions for the redesigned UI |
| djangosnippets/urls.py | Includes django_components URLs for component routing |
| djangosnippets/templates/cab/user_detail.html | Replaces table with snippet_list component and comments out sidebar |
| djangosnippets/templates/cab/user_bookmarks.html | Comments out sidebar section |
| djangosnippets/templates/cab/snippet_list.html | Replaces table with snippet_list component and comments out sidebar |
| djangosnippets/templates/cab/partials/*.html | Updates content containers and replaces tables with components |
| djangosnippets/templates/base.html | Adds Google Fonts links and implements responsive layout classes |
| cab/utils.py | Modifies object_list function to use new ObjectList class for Snippet models |
| cab/components/*.py | Implements new SnippetListComponent for rendering snippet cards |
| base/main.py | Creates ObjectList class for handling paginated object lists |
| base/components/*.py | Adds Icon component for rendering SVG icons |
Comments suppressed due to low confidence (2)
base/components/icon.html:2
- Font Awesome Free v7.0.0 does not exist. The latest major version is v6.x. This could cause licensing or attribution issues.
<svg {% html_attrs aria-label=label class=classes fill=color %} xmlns="http://www.w3.org/2000/svg" viewBox="0 0 640 640"><!--!Font Awesome Free v7.0.0 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free Copyright 2025 Fonticons, Inc.--><path d="M305 151.1L320 171.8L335 151.1C360 116.5 400.2 96 442.9 96C516.4 96 576 155.6 576 229.1L576 231.7C576 343.9 436.1 474.2 363.1 529.9C350.7 539.3 335.5 544 320 544C304.5 544 289.2 539.4 276.9 529.9C203.9 474.2 64 343.9 64 231.7L64 229.1C64 155.6 123.6 96 197.1 96C239.8 96 280 116.5 305 151.1z"/></svg>
base/components/icon.html:4
- Font Awesome Free v7.0.0 does not exist. The latest major version is v6.x. This could cause licensing or attribution issues.
<svg {% html_attrs aria-label=label class=classes fill=color %} xmlns="http://www.w3.org/2000/svg" viewBox="0 0 640 640"><!--!Font Awesome Free v7.0.0 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free Copyright 2025 Fonticons, Inc.--><path d="M192 64C156.7 64 128 92.7 128 128L128 544C128 555.5 134.2 566.2 144.2 571.8C154.2 577.4 166.5 577.3 176.4 571.4L320 485.3L463.5 571.4C473.4 577.3 485.7 577.5 495.7 571.8C505.7 566.1 512 555.5 512 544L512 128C512 92.7 483.3 64 448 64L192 64z"/></svg>
| "object_list": object_list, | ||
| "pagination": pagination, | ||
| "hits": pagination.result_count, | ||
| } | ||
| else: | ||
| context = { | ||
| "%s_list" % template_object_name: object_list, | ||
| "object_list": queryset, |
There was a problem hiding this comment.
The context key is hardcoded to 'object_list', but the template_object_name parameter suggests this should be configurable. This breaks the function's contract and could cause template variable name mismatches.
Copilot uses AI. Check for mistakes.
| "object_list": object_list, | ||
| "pagination": pagination, | ||
| "hits": pagination.result_count, | ||
| } | ||
| else: | ||
| context = { | ||
| "%s_list" % template_object_name: object_list, | ||
| "object_list": queryset, |
There was a problem hiding this comment.
The context key is hardcoded to 'object_list', but the template_object_name parameter suggests this should be configurable. This breaks the function's contract and could cause template variable name mismatches.
Copilot uses AI. Check for mistakes.
| object_list = ObjectList(request, queryset.model, queryset, 15) | ||
| pagination = object_list.pagination | ||
| else: |
There was a problem hiding this comment.
The hardcoded value 15 should use the paginate_by parameter instead. This magic number makes the code inconsistent and ignores the function's pagination configuration.
| object_list = ObjectList(request, queryset.model, queryset, 15) | |
| pagination = object_list.pagination | |
| else: | |
| paginate_by = paginate_by or 15 # Default to 15 if paginate_by is None | |
| object_list = ObjectList(request, queryset.model, queryset, paginate_by) | |
| pagination = object_list.pagination |
Copilot uses AI. Check for mistakes.
|
@chriswedgwood !! |
| <link rel="stylesheet" href="//ajax.googleapis.com/ajax/libs/jqueryui/1.11.1/themes/smoothness/jquery-ui.css" /> | ||
| <link rel="preconnect" href="https://fonts.googleapis.com"> | ||
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin> | ||
| <link href="https://fonts.googleapis.com/css2?family=Nunito:ital,wght@0,200..1000;1,200..1000&family=Playfair+Display:ital,wght@0,400..900;1,400..900&family=Raleway:ital,wght@0,100..900;1,100..900&display=swap" rel="stylesheet"> |
There was a problem hiding this comment.
@chriswedgwood
I added three google fonts, there shouldn’t be any issues right?
If there are any copyright related concerns, who should I ask to review them?
SnippetList has been redesigned ⭐️
Before
After